-
Notifications
You must be signed in to change notification settings - Fork 1
Improve JWT tests #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdded JWT structural validation to auth tests: constants for token lifetimes, helpers to decode tokens and assert payload structure, and updated login/refresh tests to verify claims (sub,type,iat,exp,jti,fresh) and expiry deltas instead of simple presence checks. Changes
Sequence Diagram(s)No sequence diagram necessary for these test-only structural assertions. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/test_auth.py (2)
13-14: Avoid hardcoding token lifetimes; derive from app config to prevent driftHardcoding 3h/3d will make tests brittle if config changes (e.g., shorter TTLs for CI). Read from Flask config with safe fallbacks.
Apply this diff to stop hardcoding at declaration:
- ACCESS_TOKEN_DELTA = 10800 # 3 hours in seconds - REFRESH_TOKEN_DELTA = 259200 # 3 days in seconds + # Set from app config at runtime (fallbacks applied in setup) + ACCESS_TOKEN_DELTA = None + REFRESH_TOKEN_DELTA = NoneAdd this helper and initialize in setup (outside the selected lines):
from datetime import timedelta def _seconds(self, val, default): if isinstance(val, timedelta): return int(val.total_seconds()) try: return int(val) except Exception: return default # In setup(), after setting self.client: with self.client.application.app_context(): cfg = self.client.application.config self.ACCESS_TOKEN_DELTA = self._seconds( cfg.get("JWT_ACCESS_TOKEN_EXPIRES", 10800), 10800 ) self.REFRESH_TOKEN_DELTA = self._seconds( cfg.get("JWT_REFRESH_TOKEN_EXPIRES", 259200), 259200 )Verification: Please confirm your test settings set JWT_ACCESS_TOKEN_EXPIRES ≈ 3h and JWT_REFRESH_TOKEN_EXPIRES ≈ 3d, or the above fallback values will be used.
62-75: Use elif to avoid accidental override; keep expiry-delta intent clearSmall logic/readability fix; the second if shouldn’t run when type == "access".
Apply this diff:
- expected_delta = None - if expected_type == "access": - expected_delta = self.ACCESS_TOKEN_DELTA - if expected_type == "refresh": - expected_delta = self.REFRESH_TOKEN_DELTA + expected_delta = None + if expected_type == "access": + expected_delta = self.ACCESS_TOKEN_DELTA + elif expected_type == "refresh": + expected_delta = self.REFRESH_TOKEN_DELTAOptional: After adopting config-driven TTLs (see Lines 13-14), this section will remain robust to future config changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/test_auth.py(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/test_auth.py (1)
tests/test_category.py (3)
test_update_category_expired_token(144-163)TestCategory(7-241)test_create_category_expired_token(121-128)
🔇 Additional comments (2)
tests/test_auth.py (2)
1-2: LGTM on math import usageImport is necessary for the isclose tolerance check below.
7-7: LGTM on decode_token importCorrect import for payload validation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit